Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add SearchAfterMixin for ES search_after capability #4536

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

Ali-D-Akbar
Copy link
Contributor

@Ali-D-Akbar Ali-D-Akbar commented Jan 8, 2025

PROD-4233
Adds a new SearchAfterMixin to be added in place of PkSearchableMixin that allows using search_after. Once this mixin is used, it will bypass the default search limit of 10k by making multiple calls to ES in case we have more than 10k records in an index.

Previously, we faced an issue regarding the search limit, resulting in less records to be returned. We increased the MAX_RESULT_WINDOW before but a better way is to use search_after capability for an optimal and flexible result.

This PR also adds a v2 CatalogQueryContainsViewSet that fixes the querying mechanism by filtering the items at the time of when we're executing queries on ES. In v1, our CatalogQueryContainsViewSet first searched all the records AND THEN filtered it.

Testing Instructions:

  1. Run update_index locally in Discovery shell.
  2. Visit /api/v1/catalog/query_contains/ endpoint and add a sample query like this: http://localhost:18381/api/v2/catalog/query_contains/?course_uuids=2de67490-f748-4efd-8532-b445f7ecc6f9,f9f1e100-668a-4fd5-a966-a127de1f69de&query=org:edX

You can set ELASTICSEARCH_DSL_QUERYSET_PAGINATION to your specific value in order to test the behavior. While the end result will be the same but this can affect the number of times the search_after mechanism is called.

@Ali-D-Akbar Ali-D-Akbar marked this pull request as ready for review January 10, 2025 09:59
@Ali-D-Akbar Ali-D-Akbar force-pushed the aakbar/PROD-4233 branch 4 times, most recently from 2b92b48 to d93f032 Compare January 13, 2025 20:30
@Ali-D-Akbar Ali-D-Akbar force-pushed the aakbar/PROD-4233 branch 2 times, most recently from de8f6a3 to e2052c4 Compare January 16, 2025 17:19
@Ali-D-Akbar Ali-D-Akbar force-pushed the aakbar/PROD-4233 branch 3 times, most recently from 66e7fae to c3c3901 Compare January 20, 2025 07:57
@Ali-D-Akbar Ali-D-Akbar force-pushed the aakbar/PROD-4233 branch 2 times, most recently from 0c21348 to eed171c Compare January 29, 2025 04:33
@Ali-D-Akbar Ali-D-Akbar merged commit 63a1ecd into master Jan 29, 2025
14 checks passed
@Ali-D-Akbar Ali-D-Akbar deleted the aakbar/PROD-4233 branch January 29, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants